Skip to content

Refactor parallel communicators, Introduce module_parallel and migrate rhog_io to domain-aware interfaces - #7899

Merged
mohanchen merged 25 commits into
deepmodeling:developfrom
mohanchen:2026-09-02-b
Sep 4, 2026
Merged

Refactor parallel communicators, Introduce module_parallel and migrate rhog_io to domain-aware interfaces#7899
mohanchen merged 25 commits into
deepmodeling:developfrom
mohanchen:2026-09-02-b

Conversation

@mohanchen

@mohanchen mohanchen commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

This PR introduces a new source_base/module_parallel framework that models each parallel communication domain as a typed subclass of ParaWorld, and migrates rhog_io as the first real-world validation target.

What's new: module_parallel

Layer Files Purpose
Base para_world.h/cpp Abstract base with rank(), size(), comm(), tag
Registry para_collection.h/cpp Owns all domains by tag
Domains para_pw/kmesh/rgrid/diag/bgroup/matrix_world.h/cpp Typed subclasses own domain-specific state
Factory para_setup.h/cpp MPI_Comm_split a parent comm into all child domains
Comm para_mpi_func.h/cpp Free-function wrappers (bcast, reduce_all, gatherv, ...) that take const ParaWorld& — no globals

Design: domain state lives in subclasses; basic communication uses free functions; cross-domain ops stay as domain methods (e.g., ParaRgridWorld::reduce_across_pools).

Worked example: rhog_io migration

Before: rhog_io.cpp read PARAM.inp.nspin and GlobalV::ofs_warning / MY_RANK implicitly via ModuleBase::WARNING, had no test coverage for write_rhog, and lived in source_io/module_chgpot/.
After:

  • PARAM.inp.nspin → explicit const int nspin parameter
  • WARNING / TITLE / timer → injected std::ostream* os_warning + ParaWorld::rank() via a local warn() helper
  • MPI_Allgatherv with raw GlobalV pool vars → para_mpi_func::gatherv(..., pw_world)
  • ModuleBase::GlobalFunc::ZEROSstd::fill
  • #define private public hack removed from test
  • PW_Basis* manual new/delete → value member + std::vector
  • Relocated to source_estate/rhog_io.{h,cpp} (owns-electrons home), namespace elecstate
  • 4 tests → 12 tests covering both read and write, nspin=1/2/4, nspin=2→4 special path, round-trip, file errors, null os_warning

Defensive validation added at entry:

if (pw_basis == nullptr) { warn(...); return false; }
if (rhog == nullptr)     { warn(...); return false; }
if (nspin != 1 && nspin != 2 && nspin != 4) { warn(...); return false; }

Governance

Two new rules in AGENTS.md:

  • Rule 10: No new #define private public access hacks in tests.
  • Rule 11: New unit tests follow test_<module_name>.cpp.

Intentionally not done

  • write_libxc_r.cpp — its GlobalV usage is about pool topology (MY_POOL, RANK_IN_POOL), a parallel-domain problem for a future pass.
  • No historical tests renamed to test_*.cpp.

Diff

82 files changed, +4149 / −765
 source_base/module_parallel/   ← new framework + 16 tests
 source_estate/rhog_io.*        ← relocated + refactored
 source_estate/test/test_rhog_io.cpp ← 12 comprehensive tests
 source_esolver/esolver_fp.cpp       ← updated call sites
 source_estate/module_charge/charge_init.cpp ← updated call sites
 source/Makefile.Objects, CMakeLists.txt     ← build wiring

Verification

  • module_parallel: 16 unit tests (6 MPI) pass.
  • Full build + integration tests to be verified in CI (sandbox lacks MPI compiler).

abacus_fixer added 12 commits September 1, 2026 21:25
Scope: 18 files changed, +75 -134 (net -59 lines). No behavior change.
Build: verified with cmake --build build (exit=0), abacus_basic_para built.

Parallel common (模板化去重)
- parallel_common.cpp: collapse the 6 per-type bcast_* copy-paste
  wrappers (int / double / complex<double>, scalar + array) into one
  template <typename T> bcast_world_impl backed by Parallel_Reduce's
  existing MPI_Type<T> traits. Keep bcast_bool/string/char as bespoke
  helpers and drop the redundant extra assignment in bcast_bool.
- test_parallel/CMakeLists.txt (MODULE_BASE_ParaCommon): list the
  transitive objects the standalone test now needs - parallel_reduce,
  parallel_comm, parallel_global, tool_quit, global_file,
  global_function, memory_recorder, timer - because parallel_common.cpp
  references Parallel_Reduce::MPI_Type<T>::value, which depends on the
  six global MPI_Comm in parallel_comm.cpp, which calls
  Parallel_Global::divide_mpi_groups.

Parallel 2D (收紧头文件依赖, rule 3)
- parallel_2d.h: drop the unused #include "source_base/parallel_comm.h"
  (parallel_2d.h only needed <mpi.h> for MPI_Comm); this was the single
  largest conduit that pulled POOL_WORLD/KP_WORLD/INT_BGROUP/BP_WORLD/
  GRID_WORLD/DIAG_WORLD declarations into every user of Parallel_2D /
  Parallel_Orbitals.
- Explicitly include source_base/parallel_comm.h in the 13 consumers
  that were relying on transitive include to reference the global
  communicators: write_hs.h, hsolver_lrtd.hpp, sto_iter.cpp /
  sto_tool.cpp / sto_dos.cpp / sto_elecond.cpp, chgmixing.cpp,
  hsolver_pw_sdft.cpp, esolver_sdft_pw.cpp, diago_bpcg_test.cpp,
  test_hsolver_sdft.cpp.

Parallel grid (重复逻辑收敛 + 现代C++清理)
- parallel_grid.h/cpp: merge zpiece_to_all and zpiece_to_stogroup into
  one zpiece_distribute(zpiece, iz, rho, is_sdft). The only
  difference between the two (~130 lines each) is the choice of
  communicator (MPI_COMM_WORLD vs INT_BGROUP) and the root rank used
  in the non-pool-0 receive path (MY_RANK vs RANK_IN_BPGROUP); both
  are selected with two local variables so the four send/recv
  branches (pool0 root copy, other-rank recv, pool-root multicast,
  other-pool recv) share one implementation. Also rename duplicate
  "case 2" labels into "case 2 / case 3".
- parallel_grid.cpp::z_distribution: replace raw new int[KPAR] /
  delete[] startp with std::vector<int> startp(KPAR) and remove
  five blocks of commented-out debug output.

Misc dead-code / include cleanup
- parallel_reduce.h: remove the dead declaration
  bool check_if_equal(double& v) - never defined, never referenced
  anywhere in the repo.
- parallel_global.cpp: drop two unused includes (parallel_common.h,
  parallel_reduce.h) left over from earlier refactors.

Governance notes:
- GlobalV budget: PR total added=3 GlobalV refs, removed=19,
  net_delta = -16. The 3 new refs are inside the merged
  zpiece_distribute function (it uses the same GlobalV::MY_POOL etc.
  as the original two functions, they just appear on new lines in the
  diff). Remaining GlobalV usage in Parallel_Grid stays for Step 1
  (ProcessTopology injection).
- Added header includes: parallel_2d.h now includes <mpi.h> directly
  (it previously got MPI_Comm via parallel_comm.h); write_hs.h and
  hsolver_lrtd.hpp now include parallel_comm.h because the
  implementations reference DIAG_WORLD and POOL_WORLD respectively
  and were previously hiding that dependency behind the Parallel_Orbitals
  -> Parallel_2D -> parallel_comm transit.
- No INPUT / documentation change required: all public APIs keep the
  same signatures and semantics (bcast, grid reduce, Parallel_2D).
…n, ParaKmeshWorld

Step 1-2: establish module_parallel/ as the new home for parallel domain abstractions.

- ParaWorld: base class with tag + rank + size + comm, virtual destructor
  for polymorphism, protected constructors, make_serial factory for
  ParaCollection::add()
- ParaTag: 8 domain tag string constants (pw/kmesh/bsame_kdiff/bdiff_ksame/
  rgrid/diag/matrix/atom)
- ParaCollection: unique_ptr<ParaWorld> container with find(tag) and
  find_as<T>() for safe downcast; missing tag returns static empty domain
- ParaKmeshWorld: first domain subclass, extracts Parallel_Kpoints logic
  (k-point distribution, pool mapping, cross-pool collection, gather_kvec)
  into a self-contained class; tests only include para_kmesh_world.h
- All tests moved to module_parallel/test/ with both serial and MPI variants
- Old para_world tests in test/ and test_parallel/ removed
- CMakeLists.txt and Makefile.Objects updated
Extract pool-level parallel parameters (poolnproc, poolrank, npw, npw_per,
npwtot) from PW_Basis into a self-contained domain class. The actual
FFT-based distribution algorithm (method1/method2) stays in PW_Basis;
ParaPwWorld only holds the result: how many plane waves each process gets.

Tests only include para_pw_world.h, no PW_Basis or parallel_comm.h needed.
Extract DIAG_WORLD + GlobalV::DRANK/DSIZE/DCOLOR into a self-contained
domain class with drank()/dsize()/dcolor() aliases. Used by PEXSI
solver and LCAO HS IO modules.
Extract GRID_WORLD + GlobalV::GRANK/GSIZE + Parallel_Grid's
z-distribution tables (numz/startz/whichpro) into a self-contained
domain class. Owns grid dimensions and per-process z-plane allocation.

Cross-pool operations (reduce_across_pools, bcast, reduce) will be
added later with explicit communicator parameters, breaking the
cross-domain dependency on KP_WORLD/INT_BGROUP.
Extract INT_BGROUP + BP_WORLD + GlobalV::MY_BNDGROUP/NPROC_IN_BNDGROUP/
RANK_IN_BPGROUP into a self-contained domain class with both intra
and inter group communicators. Used by SDFT band parallel and BPCG
diagonalization.
Extract the process-grid part of Parallel_2D (dim0/dim1/coord) into a
self-contained domain class. The actual ScaLAPACK descriptor and BLACS
context management stay in Parallel_2D; this class only holds the
process grid dimensions and coordinates, computed automatically from
the communicator size.
Add para_comm.h/.cpp providing domain-aware communication primitives
(bcast, reduce, gather, min/max) that accept const ParaWorld& instead
of hardcoding MPI_COMM_WORLD or POOL_WORLD. Serial mode is a no-op
(except gather_int which copies locally). Invalid worlds are skipped.

Tests only need para_comm.h + para_world.h, no dependency on the old
parallel_common.h or parallel_reduce.h.
Add reduce_across_pools, bcast_data, and reduce_data methods to
ParaRgridWorld. These replace Parallel_Grid::reduce_across_pools,
Parallel_Grid::bcast, and Parallel_Grid::reduce respectively.

Key difference from old code: communicators are passed explicitly
as const ParaWorld& parameters, eliminating GlobalV::KPAR/MY_POOL/
RANK_IN_POOL and global POOL_WORLD/KP_WORLD/INT_BGROUP dependencies.

Serial mode: reduce_across_pools is no-op, bcast_data/reduce_data
copy local slabs directly. Invalid worlds are skipped safely.
Add para_setup.h/.cpp providing:
- divide_mpi_groups: utility to split nproc into num_groups (serial+MPI)
- split_pools: split WORLD into k-pools + band groups, returns ParaWorld
  objects for pw/kmesh/bsame_kdiff/bdiff_ksame domains
- split_diag_world: split for DIAG_WORLD
- split_grid_world: split for GRID_WORLD
- setup_para_worlds: top-level function assembling all 8 domains into
  a ParaCollection

Also adds make_mpi/make_mpi_ptr factory methods to ParaWorld for
constructing domains from MPI communicators (protected ctor stays
protected, factories are the public API).

Replaces Parallel_Global::divide_pools, split_diag_world,
split_grid_world, and divide_mpi_groups without touching old code.
The name "para_comm" was ambiguous (communicator? communication?).
Rename to para_mpi_func to match the file's role: a collection of
domain-aware MPI functions (bcast/reduce/gather/min/max).

Files renamed:
- para_comm.h/.cpp -> para_mpi_func.h/.cpp
- para_comm_test.cpp -> para_mpi_func_test.cpp
- para_comm_mpi_test.cpp/.sh -> para_mpi_func_mpi_test.cpp/.sh

Targets renamed accordingly (MODULE_BASE_para_mpi_func[_mpi]);
gtest suites renamed to ParaMpiFuncTest/ParaMpiFuncMpiTest.
No behavior change, 16/16 tests pass.
Add two top-level parallel domains for multi-image calculations
(e.g. NEB replicas with independent unit cells):

- para_esolver_world [color = image_id]: all processes of one
  esolver instance; every existing solver domain is now derived
  from it instead of being hard-wired to MPI_COMM_WORLD.
- para_images_world [color = rank_in_esolver]: cross-image
  communicator connecting corresponding ranks; MPI_COMM_NULL for
  nimage == 1 or uneven splits, following the KP_WORLD convention.

split_pools/split_diag_world/split_grid_world now take a parent
MPI_Comm, and setup_para_worlds takes nimage and builds the full
hierarchy: WORLD -> images split -> esolver domain -> kmesh/pw/
bgroup/diag/rgrid/matrix. The hierarchy tree is documented in
para_setup.h.

New MPI test (mpirun -np 4) verifies single-image degradation,
2-image split ranks/sizes, derived-domain sizes, and kpar=2 inside
images; multi-image tests skip under single-rank direct execution.
@mohanchen mohanchen added Refactor Refactor ABACUS codes The Absolute Zero Reduce the "entropy" of the code to 0 labels Sep 3, 2026
End-to-end pilot migration of read_rhog/write_rhog from the old
POOL_WORLD + GlobalV::RANK_IN_POOL + #ifdef __MPI pattern to the new
ParaWorld-based interface.

Changes:
- Add barrier() to para_mpi_func (required by write_rhog's 6 barriers)
- Add para_bridge.h/.cpp: temporary factory make_pw_world() that wraps
  POOL_WORLD into a ParaWorld, hiding #ifdef __MPI from call sites.
  Delete this file once ParaCollection is wired into driver init.
- Migrate rhog_io.cpp: all MPI_Bcast(POOL_WORLD) -> Parallel::bcast_*,
  all MPI_Barrier(POOL_WORLD) -> Parallel::barrier, all
  GlobalV::RANK_IN_POOL -> pw_world.rank(). Zero #ifdef __MPI in the
  file body.
- Change read_rhog signature: add const ParaWorld& parameter.
- Change write_rhog signature: replace ipool/irank/nrank with
  const ParaWorld&. Move "only pool 0 writes" check to caller.
- Update call sites: charge_init.cpp (2 read_rhog calls),
  esolver_fp.cpp (2 write_rhog calls), read_rhog_test.cpp.
- Add barrier tests to para_mpi_func_test and para_mpi_func_mpi_test.

Verification: make -j30 0 errors; ctest read_rhog 4/4 pass,
module_parallel 18/18 pass. Old parallel_* test failures are
pre-existing (missing .sh scripts).
@mohanchen
mohanchen requested a review from Qianruipku September 3, 2026 06:49
abacus_fixer added 5 commits September 3, 2026 14:50
Without this VPATH entry, make cannot locate para_*.cpp source files
for the OBJS_PARALLEL targets, causing build failure:
  No rule to make target 'build/obj/para_world.o'
- Add explicit nspin parameter to read_rhog() signature, making it
  consistent with write_rhog() which already has nspin
- Remove #include of parameter.h and global_variable.h from rhog_io.cpp
  - No longer reads PARAM.inp.nspin anywhere
- Update charge_init.cpp call sites to pass the existing local nspin var
- Remove dead PARAM.input.nspin assignments and #define private public
  hack from read_rhog_test.cpp (rhog_io no longer consumes PARAM)
- PW_Basis* rhopw (new+delete) -> value member PW_Basis rhopw
- new complex*[1] + new complex[1471] (double-delete chain)
  -> vector<vector<complex>> rhog_data + vector<complex*> rhog
- Remove TearDown() entirely (RAII handles cleanup)
- Update call sites to use &rhopw and rhog.data()
- Add std::ostream* os_warning parameter to read_rhog() and write_rhog()
  (no default value; callers must pass explicitly)
- Replace ModuleBase::WARNING/WARNING_QUIT with a local warn() helper
  that writes to the injected stream on rank 0
- Remove indirect GlobalV dependencies: drop TITLE, timer, WARNING,
  WARNING_QUIT calls; drop timer.h include
- write_rhog: replace WARNING_QUIT with warn() + return false (same flow)
- Update all call sites to pass &GlobalV::ofs_warning:
  - charge_init.cpp (2 read_rhog calls)
  - esolver_fp.cpp (2 write_rhog calls)
  - read_rhog_test.cpp (4 read_rhog calls)
- read_rhog_test.cpp main(): replace GlobalV parallel vars with locals
…state

Move three file pairs from source_io/module_chgpot to source_estate root:
- rhog_io.h/cpp
- write_elecstat_pot.h/cpp
- write_init.h/cpp

Also migrate read_rhog_test.cpp to source_estate/test/.

Update all include paths, CMakeLists.txt (source_io and source_estate,
including test configs), and Makefile.Objects (OBJS_IO -> OBJS_SRCPW).

Add AGENTS.md rules 10-11: forbid #define private public access hacks
in new tests, and require test_<module>.cpp naming for new unit tests.
abacus_fixer and others added 5 commits September 3, 2026 16:36
- Add 3 new tests: WriteRoundTrip, WriteFileFail, WriteRoundTripNspin2
  (write_rhog was previously 0% covered by tests)
- Add OsNullptrSilent test for nullptr warning stream safety
- Remove GlobalV::ofs_warning usage, use local std::ofstream + fixture helpers
- Extract setup_pw_basis() fixture method to avoid duplication
…ill, add input validation

- Rename namespace ModuleIO -> elecstate to match source_estate convention
  (rhog_io was the only source_estate file still using ModuleIO)
- Replace 3x ModuleBase::GlobalFunc::ZEROS with std::fill, remove
  global_function.h dependency, add <algorithm>
- Add defensive parameter checks at the top of read_rhog and write_rhog:
  - pw_basis null check
  - rhog null check
  - nspin range check (1-4)
  - read_rhog: nx/ny/nz > 0 check
- Update all call sites in esolver_fp.cpp, charge_init.cpp, test_rhog_io.cpp
- Update test expected warning strings to match new namespace
@mohanchen mohanchen changed the title Refactor parallel communicators, try 2 Refactor parallel communicators, Introduce module_parallel and migrate rhog_io to domain-aware interfaces Sep 3, 2026
The test aborted with "MPI_Comm_rank() called before MPI_INIT" because the
directory-level abacus_disable_feature_definitions(__MPI) in
source_estate/test stripped __MPI from every target there. With __MPI gone,
the test's main() never called MPI_Init, while the linked base/planewave
libraries still issue real MPI calls.

Add a target-level escape hatch: a new ABACUS_KEPT_FEATURE_DEFINITIONS
target property (exposed as AddTest's KEEP_FEATURE_DEFINITIONS) lets a
single target keep definitions its directory disables. Use it for this
test, link planewave instead of planewave_serial, and copy the support/
data at configure time (install() is skipped by plain make+ctest).

Also fix test bugs hidden while __MPI was stripped: drop const from kpar
(passed by reference to divide_pools), call setup_pw_basis() in the three
tests that need a valid grid, and set npwtot=1000 in InconsistentGammaOnly
to trigger the expected "planewaves not used" warning.

Verified: make MODULE_ESTATE_test_rhog_io && ctest -R
MODULE_ESTATE_test_rhog_io -> 10/10 passed.
charge_mpi_test is a real MPI test (its main() calls MPI_Init) but does not
need a separate directory now that AddTest supports KEEP_FEATURE_DEFINITIONS.
Move it into test/ alongside the other module tests, keep __MPI for that
target, and register its mpirun -np 4 variant there. Remove the now-empty
test_mpi/ subdirectory and its add_subdirectory() call.

Verified: ctest -R "MODULE_ESTATE_charge_mpi_test|MODULE_ESTATE_test_rhog_io"
-> 3/3 passed (test_rhog_io 10/10, charge_mpi_test, charge_mpi_test_4np 4/4).
@mohanchen
mohanchen merged commit ee450d9 into deepmodeling:develop Sep 4, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Refactor Refactor ABACUS codes The Absolute Zero Reduce the "entropy" of the code to 0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants